Skip to content

Modmenu pagination#123

Open
Aurumbi wants to merge 9 commits intoAWSW-Modding:developfrom
Aurumbi:modmenu-pagination
Open

Modmenu pagination#123
Aurumbi wants to merge 9 commits intoAWSW-Modding:developfrom
Aurumbi:modmenu-pagination

Conversation

@Aurumbi
Copy link

@Aurumbi Aurumbi commented Feb 17, 2026

Added pagination to the in-game mod browser, along with fixing certain issues regarding to getting modlists from steam.

Main Changes

Fixed steamhandler's QueryApi method

This method, which is used to get sections of the modlist from steam's API, has a tendency to crash silently on repeated calls, which also crashes the game. The source of this crash seems to be related to steam caching these results, as these crashes persist between play sessions, and occur consistently from the second call onwards for about 15 minutes after a successful call.
This was solved by created an on-disk cache of this method's results, and using that cache until that 15-minute interval elapses, which allows the QueryApi calls to never use the problematic steam cache.

Added pagination to the In-game mod browser

With the increasing number of mods available in the game since the development on the browser, both load time and navigation go slower as a consequence of the longer modlist. pagination was then added to both allow partial loading of the modlist, and to allow quicker navigation of the modlist.

  • note: While partial loading is now possible, It has not been implemented. with the loading improvements implemented in the next section, it has been deemed not necessary.

Load speed improvements

The critical path of getting the modlist (which defines the content of the browser) has been examined, and certain optimizations have been made. of those there are:

  1. The modlist is preloaded and cached on startup, which saves significant time on the first opening of such list.
  2. Certain debugging information has been reduced to it's identifiers, significantly reducing IO.

mods[-1][3] += "\n\nVerified by {}".format(verified.username.replace("<postmaster@example.com>", ""))
else:
print "NOT VALID SIG", mod
print "NOT VALID SIG", mod[1] # Note: printing only the mod name, instead of the whi=ole thing SIGNIFICANTLY speeds up this call
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly worrying - how many mods don't have a valid signature? I thought I'd signed most of them

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 15 fail, though I haven't checked why

(3336106514) ALL WORKING MODS! 32 (Flight of Love doesn't show)
(3651582794) Angels With Scaly Wings: Chinese Simplified
(3666234554) Card Game Expanded 卡牌游戏增强(Two Languages Support)
(2801825863) Casual Arson
(2697982171) Casual Vandalism
(2096774388) Lorem_RPG
(2742645268) MagmaClient
(2990207385) Meet Naomi
(1305731599) Modtools
(2766323849) Name Re-entry
(3465212790) Naomi Maze Skip
(2736325690) Skip Credits
(2987066957) The Morning After
(3645564443) The Traitor
(3657449256) Vykoupení

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of those I can't remember anyone asking to sign, though I have signed 2 recently. The modtools itself is actually never signed so that's fine

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's fair. I'm still hesitant to return the printing to how it was, As it seems to double the runtime of the validity check (from ~2 seconds to ~5 seconds)...
As for the modtools, I'll add a special case for it (to be ignored)

# else: already done

# Write cache file
with open(cache_file_name, "wb") as cache_file:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use literally anything except pickle? It's very insecure.

Copy link
Author

@Aurumbi Aurumbi Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I used pickle because it was the simplest way to store and restore values so that they behave the same, but I have no attachment to it. I'm afraid I don't quite get what the security issue here is, so what would you suggest to use here? would a tuple of dicts be better (with adapted __getattr__ so that they behave like the ctype results)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Loading pickles can result in arbitrary code execution, but I guess the mod ecosystem is full of that. I'd still avoid it if we can. Why not say json?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot pickle can do that... json is seems to be the most standard way of dealing with this kind of data, so I guess we'll use that

if not isinstance(steam_manager, SteamMgr):
raise TypeError("steam_manager must be a steam_workshop.steamhandler.SteamMgr instance!")
self._steam_manager = steam_manager
self.threads = [] # This can create threads in QueryApi, which will persist after the call returns (as is required).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list only grows, never shrinks. Is this AI written?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was mostly not thought out well... Because of how QueryApi (and all of steam DLL functions) work, if the cache file is used then we need to create a thread to execute the query callbacks in order to imitate the behavior of steamhandler.QueryApi. These threads also need to live beyond the end of the function. I was concerned that these threads could be difficult to track (especially if some of the callbacks start misbehaving), so I thought "why not just chuck them into a list, so that we at least know which ones have been created?". but then I didn't really have much to do with them, so the list became kinda pointless.
Additionally, I avoid using AI for my work, so any stupid decisions are my own.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the thread at least should self-clean itself up when it's done?

Copy link
Author

@Aurumbi Aurumbi Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I should at least remove them from the list... as the threads themselves shouldn't have anything internal to clean... the multiprocessing ThreadPool would have been ideal, but multiprocessing is apparently not available, so I guess I'll just have them remove themselves when done.

print "Called cached queryAPI with page={}".format(page)

# Get most recent cache file if exists
cache_file_name = self.get_cache_filename(page)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we storing a cache in a file rather than say in memory?
Given that it expires in 15 minutes, that's not particularly useful for multiple play sessions

Copy link
Author

@Aurumbi Aurumbi Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue I was running into (which was the main one preventing the game from starting) Was that repeated calls to QueryApi failed silently, and in doing so, crashed the game. Once these start to fail, they keep failing even between play sessions until about 15 minutes pass, which prevents anything from accessing the modlist until that time passes. Therefore, I use a file to store those results, as they need to be accessible between play sessions.
I've added a description of this in the PR's description, as this is the most significant change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's really weird and I'm not sure what to make of it to be honest

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I only found it by trying to isolate the issues with CachePersonas (mainly why it crashes silently and abruptly), and found it really weird that QueryApi itself can cause it...
I do wonder if it happens just on my machine of is it a more widespread thing. I'd appreciate it if you can check if it also happens on yours (I did so by replacing the content of CachePersonas with a single call to QueryApi(1). Then I started the game, quit out of the main menu, then tried to start it again, which failed by this issue.).
It'd be quite interesting if it's just a localized thing on my machine...

cPickle.dump(tuple(islice(array, arr_len)), cache_file)

except Exception as e:
print "Cache callback raised Exception:", str(e)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why just printing here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't remember, I think it's a holdover from testing? We can reraise, or just remove the except case altogether.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just try/finally is probably fine here if there's nothing we actually want to handle

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this callback fails I think it should be logged though? As it's a call that shouldn't fail under normal circumstances, and it's failure can be very problematic... But I guess I should still say as such in the code, as it currently seems quite random

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the callback fails, it should probably cause an error screen to show up right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so. is modloader.report_mod_errors appropriate for this case?


from modloader import modconfig

# Cache mod validity to expedite mod browser startup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to modconfig.steam_downloadable_mods still takes a few seconds on the first call (as after that it's cached in memory), so I chucked it on a thread during startup so that it would already be done before the user ever wants to access the mod browser.
A more robust way of doing this would probably be to make a load_steam_downloadable_mods function which loads this data on a thread, then setup steam_downloadable_mods so that it waits until that function finishes. I honestly should have thought of that sooner...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants